Conversation
WalkthroughA refund management feature was introduced to the admin module. This includes a new paginated GET endpoint for refunds, a service method to retrieve refund data, a repository interface and its implementation for querying refunds, and a DTO for refund listings. The database schema update strategy was also changed to avoid dropping data on restart. Additionally, a utility method for date formatting and a Lombok getter annotation for payment method enum were added. Several domain and DTO classes were updated to use string representations instead of enums for better clarity and consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUser
participant AdminController
participant AdminService
participant RefundQueryRepositoryImpl
participant Database
AdminUser->>AdminController: GET /admin/refunds?page=&size=
AdminController->>AdminService: getRefunds(pageable)
AdminService->>RefundQueryRepositoryImpl: searchAllRefunds(pageable)
RefundQueryRepositoryImpl->>Database: Query refund data with joins and pagination
Database-->>RefundQueryRepositoryImpl: Return refund data
RefundQueryRepositoryImpl-->>AdminService: Page<RefundListResponse>
AdminService-->>AdminController: Page<RefundListResponse>
AdminController-->>AdminUser: 200 OK + ApiResponseWrapper<Page<RefundListResponse>>
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @jbh010204, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new administrative feature that enables the viewing of a paginated list of refund requests. It involves adding a new API endpoint, service logic, and a dedicated QueryDSL-based repository to efficiently fetch and present detailed refund information by joining various related entities.
Highlights
- New Admin Refund API: A new
/admin/refundsendpoint has been added to allow administrators to retrieve a paginated list of refund requests. This endpoint acceptspageandsizeparameters for pagination. - Refund Query Service and Repository: A new
getRefundsmethod is introduced inAdminService, which leverages a newly createdRefundQueryRepositoryand itsRefundQueryRepositoryImplimplementation. This repository uses QueryDSL to perform complex joins across refund, application, profile, and payment entities to gather comprehensive refund details. - Refund List DTO: A new
RefundListResponserecord is defined to structure the data returned for each refund entry, including details like refund ID, examination number, user name, phone number, request/completion dates, payment method, and reason. - Database Schema Management Change: The
ddl-autosetting inapplication.ymlhas been changed fromcreate-droptoupdate. This means Hibernate will attempt to update the database schema based on entity changes rather than dropping and recreating it on each application startup.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request implements a new feature for administrators to view a list of refunds. The implementation adds a new controller endpoint, service method, and a QueryDSL-based repository to fetch the data.
I found a critical security issue where an admin endpoint is left unprotected. The pagination implementation is incomplete, and I've provided suggestions to return a Page object. There are also some minor code style suggestions and a high-risk configuration change in application.yml regarding hibernate.ddl-auto.
| } | ||
|
|
||
| @GetMapping("refunds") | ||
| // @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") |
There was a problem hiding this comment.
The @PreAuthorize annotation is commented out. This exposes an admin endpoint to any user, which is a critical security vulnerability. Please uncomment this line to ensure only authenticated administrators can access this endpoint.
| // @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") | |
| @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") |
| public List<RefundListResponse> getRefunds(int page, int size) { | ||
| Pageable pageable = PageRequest.of(page, size); | ||
| return refundQueryRepository.searchAllRefunds(pageable); | ||
| } |
There was a problem hiding this comment.
To support proper pagination as suggested for the AdminController, this method should accept a Pageable object and return a Page<RefundListResponse>. This aligns with how other paginated methods in this service are implemented and simplifies the controller-service interface.
public Page<RefundListResponse> getRefunds(Pageable pageable) {
return refundQueryRepository.searchAllRefunds(pageable);
}|
|
||
| public interface RefundQueryRepository { | ||
|
|
||
| List<RefundListResponse> searchAllRefunds(Pageable pageable); |
There was a problem hiding this comment.
To support proper pagination, this method should return Page<RefundListResponse> instead of List<RefundListResponse>. This will allow you to provide total element count and other pagination information to the client.
| List<RefundListResponse> searchAllRefunds(Pageable pageable); | |
| org.springframework.data.domain.Page<RefundListResponse> searchAllRefunds(Pageable pageable); |
| public List<RefundListResponse> searchAllRefunds(Pageable pageable) { | ||
| JPAQuery<Tuple> query = baseQuery() | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()); | ||
|
|
||
| return query.fetch().stream() | ||
| .map(this::mapToResponse) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
To implement pagination correctly, this method should return a Page<RefundListResponse>. This involves two steps: fetching the list of results for the current page, and fetching the total count of all results. You can then use PageImpl to construct the Page object.
@Override
public org.springframework.data.domain.Page<RefundListResponse> searchAllRefunds(Pageable pageable) {
List<RefundListResponse> content = baseQuery()
.offset(pageable.getOffset())
.limit(pageable.getPageSize())
.fetch()
.stream()
.map(this::mapToResponse)
.toList();
JPAQuery<Long> countQuery = queryFactory
.select(refund.count())
.from(refund);
return new org.springframework.data.domain.PageImpl<>(content, pageable, countQuery.fetchOne());
}| public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefundCounts( | ||
| @RequestParam(defaultValue = "0") int page, | ||
| @RequestParam(defaultValue = "15") int size | ||
| ) { | ||
| List<RefundListResponse> result = adminService.getRefunds(page, size); | ||
| return ResponseEntity.ok( | ||
| ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result)); | ||
| } |
There was a problem hiding this comment.
The method getRefundCounts is misleading as it returns a list of refunds, not just counts. It should be renamed to getRefunds.
For proper pagination, it should return a Page<RefundListResponse> which includes total element count and total pages. This is also inconsistent with other paginated endpoints in this controller like getStudents, which already use Pageable and return Page.
The success message "환불 신청 수 조회 성공" (refund application count inquiry success) is also misleading and should be updated to reflect that a list is being returned, e.g., "환불 신청 목록 조회 성공" (refund application list inquiry success).
public ResponseEntity<ApiResponseWrapper<Page<RefundListResponse>>> getRefunds(
Pageable pageable
) {
Page<RefundListResponse> result = adminService.getRefunds(pageable);
return ResponseEntity.ok(
ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 목록 조회 성공", result));
}| show-sql: true | ||
| hibernate: | ||
| ddl-auto: create-drop | ||
| ddl-auto: update |
There was a problem hiding this comment.
Setting spring.jpa.hibernate.ddl-auto to update is risky and not recommended for production or even shared development environments. It can lead to data loss or an inconsistent database schema, as it does not handle all types of schema changes gracefully (e.g., renaming columns).
It is highly recommended to use a database migration tool like Flyway or Liquibase to manage schema changes in a controlled, versioned, and reliable manner. If this configuration is intended only for a specific local development profile, consider moving it to a profile-specific file (e.g., application-dev.yml) and using validate or none for other environments.
| QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | ||
| QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | ||
| QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | ||
| QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | ||
| QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; |
There was a problem hiding this comment.
These QueryDSL Q-type fields are effectively constants. It's a good practice to declare them as private static final to make it clear they are stateless, shared, and immutable.
| QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | |
| QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | |
| QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | |
| QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | |
| QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; | |
| private static final QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | |
| private static final QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | |
| private static final QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | |
| private static final QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | |
| private static final QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; |
| ); | ||
| } | ||
|
|
||
| private String formatDate(java.time.LocalDateTime dateTime) { |
There was a problem hiding this comment.
The formatDate method does not depend on any instance state of RefundQueryRepositoryImpl. It can be made static to clarify that it's a stateless utility method.
private static String formatDate(java.time.LocalDateTime dateTime) {![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/RefundListResponse.java (1)
5-17: Consider timestamp handling and nullable field documentation.The record design is clean and appropriate for the use case. However, consider these improvements:
Timestamp Fields: Using
StringforrequestedAtandcompletedAtmay lead to inconsistent date formatting. Consider usingLocalDateTimeorInstantfor better type safety.Nullable Fields: Some fields like
completedAtmight be null for pending refunds. Consider adding documentation or usingOptional<T>for clarity.public record RefundListResponse( Long refundId, String examNumber, String name, String phone, - String requestedAt, - String completedAt, + LocalDateTime requestedAt, + LocalDateTime completedAt, PaymentMethod paymentMethod, String reason ) {This would require updating the repository implementation to handle LocalDateTime formatting.
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java (1)
1-75: Consider architectural placement and pagination completeness.Regarding your question about domain placement: The current approach of placing QueryRepository in the admin domain is reasonable since this is admin-specific functionality. However, consider these alternatives:
- Current approach (admin domain): Good for admin-specific queries
- Respective domain folders: Better for reusability across different contexts
For this refund query that spans multiple domains, the admin placement is appropriate.
Additionally, consider implementing a count query method for complete pagination support:
public long countAllRefunds() { return queryFactory .select(refund.count()) .from(refund) .leftJoin(appSchool).on(refund.applicationSchoolId.eq(appSchool.id)) .fetchOne(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/life/mosu/mosuserver/application/admin/AdminService.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/RefundListResponse.java(1 hunks)src/main/resources/application.yml(1 hunks)
🔇 Additional comments (3)
src/main/resources/application.yml (1)
31-31: Database schema strategy change is appropriate for production.The change from
create-droptoupdateis necessary to preserve data in production environments. However, ensure this aligns with your deployment and migration strategy.Please verify that this change is consistent with your deployment strategy and consider:
- Do you have database migration scripts for schema changes?
- Is this change intended for all environments (dev, staging, prod)?
- Consider using environment-specific profiles if different behaviors are needed per environment.
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java (2)
40-57: The join logic looks correct and comprehensive.The left joins properly traverse the relationships to gather all necessary refund information. The join chain from refund → applicationSchool → application → profile and the separate join to payment is well-structured.
59-74: Mapping and date formatting implementation is solid.The tuple mapping correctly handles all fields and the date formatting utility properly handles null values while converting LocalDateTime to a readable string format.
| public interface RefundQueryRepository { | ||
|
|
||
| List<RefundListResponse> searchAllRefunds(Pageable pageable); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider package location and return type consistency.
The interface design is clean, but consider these architectural concerns:
-
Package Location: As mentioned in the PR description, consider if this should be in the respective domain folder instead of
admindomain. -
Return Type: The method returns
List<RefundListResponse>instead ofPage<RefundListResponse>, which is inconsistent with pagination patterns. Consider returningPage<T>to include pagination metadata.
-List<RefundListResponse> searchAllRefunds(Pageable pageable);
+Page<RefundListResponse> searchAllRefunds(Pageable pageable);This would provide total count, page information, and better align with Spring Data patterns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface RefundQueryRepository { | |
| List<RefundListResponse> searchAllRefunds(Pageable pageable); | |
| } | |
| public interface RefundQueryRepository { | |
| Page<RefundListResponse> searchAllRefunds(Pageable pageable); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepository.java
around lines 7 to 10, move the interface to the appropriate domain package
instead of the admin domain to better reflect its responsibility. Also, change
the return type of the searchAllRefunds method from List<RefundListResponse> to
Page<RefundListResponse> to include pagination metadata and align with Spring
Data pagination conventions.
src/main/java/life/mosu/mosuserver/application/admin/AdminService.java
Outdated
Show resolved
Hide resolved
| @GetMapping("refunds") | ||
| // @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") | ||
| public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefundCounts( | ||
| @RequestParam(defaultValue = "0") int page, | ||
| @RequestParam(defaultValue = "15") int size | ||
| ) { | ||
| List<RefundListResponse> result = adminService.getRefunds(page, size); | ||
| return ResponseEntity.ok( | ||
| ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address multiple consistency and naming issues.
Several concerns with this endpoint implementation:
-
Method Name Mismatch: Method is named
getRefundCountsbut returns refund details, not counts. -
Missing Path Prefix: The endpoint path should be
/refundsinstead ofrefundsto match other endpoints. -
Inconsistent Pagination: Other endpoints use
Pageableparameter, this uses individual@RequestParam. This creates API inconsistency. -
Authorization: Like other endpoints, authorization is commented out. Ensure this is intentional for development.
For consistency with other endpoints:
-@GetMapping("refunds")
+@GetMapping("/refunds")
// @PreAuthorize("isAuthenticated() and hasRole('ADMIN')")
-public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefundCounts(
- @RequestParam(defaultValue = "0") int page,
- @RequestParam(defaultValue = "15") int size
+public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefunds(
+ Pageable pageable
) {
- List<RefundListResponse> result = adminService.getRefunds(page, size);
+ List<RefundListResponse> result = adminService.getRefunds(pageable);
return ResponseEntity.ok(
ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result));
}This requires updating the service method signature as well.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("refunds") | |
| // @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") | |
| public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefundCounts( | |
| @RequestParam(defaultValue = "0") int page, | |
| @RequestParam(defaultValue = "15") int size | |
| ) { | |
| List<RefundListResponse> result = adminService.getRefunds(page, size); | |
| return ResponseEntity.ok( | |
| ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result)); | |
| } | |
| @GetMapping("/refunds") | |
| // @PreAuthorize("isAuthenticated() and hasRole('ADMIN')") | |
| public ResponseEntity<ApiResponseWrapper<List<RefundListResponse>>> getRefunds( | |
| Pageable pageable | |
| ) { | |
| List<RefundListResponse> result = adminService.getRefunds(pageable); | |
| return ResponseEntity.ok( | |
| ApiResponseWrapper.success(HttpStatus.OK, "환불 신청 수 조회 성공", result)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/presentation/admin/AdminController.java
around lines 101 to 111, rename the method from getRefundCounts to getRefunds to
reflect that it returns refund details, not counts. Update the @GetMapping
annotation to use the path "/refunds" with a leading slash for consistency.
Replace the individual page and size @RequestParam parameters with a single
Pageable parameter to align with other endpoints' pagination style. Finally,
review the commented-out @PreAuthorize annotation and either enable it or
confirm it is intentionally disabled for development; if enabling, ensure the
authorization expression matches other admin endpoints. Also, update the
corresponding service method signature to accept Pageable instead of separate
page and size parameters.
| public List<RefundListResponse> searchAllRefunds(Pageable pageable) { | ||
| JPAQuery<Tuple> query = baseQuery() | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()); | ||
|
|
||
| return query.fetch().stream() | ||
| .map(this::mapToResponse) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add ordering to ensure predictable pagination results.
The query lacks ordering which can lead to unpredictable pagination results. Consider adding an order by clause, typically by creation date or ID.
public List<RefundListResponse> searchAllRefunds(Pageable pageable) {
JPAQuery<Tuple> query = baseQuery()
+ .orderBy(refund.createdAt.desc())
.offset(pageable.getOffset())
.limit(pageable.getPageSize());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public List<RefundListResponse> searchAllRefunds(Pageable pageable) { | |
| JPAQuery<Tuple> query = baseQuery() | |
| .offset(pageable.getOffset()) | |
| .limit(pageable.getPageSize()); | |
| return query.fetch().stream() | |
| .map(this::mapToResponse) | |
| .toList(); | |
| } | |
| public List<RefundListResponse> searchAllRefunds(Pageable pageable) { | |
| JPAQuery<Tuple> query = baseQuery() | |
| .orderBy(refund.createdAt.desc()) | |
| .offset(pageable.getOffset()) | |
| .limit(pageable.getPageSize()); | |
| return query.fetch().stream() | |
| .map(this::mapToResponse) | |
| .toList(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java
around lines 30 to 38, the searchAllRefunds method's query lacks an order by
clause, causing unpredictable pagination results. Add an orderBy clause to the
JPAQuery, typically ordering by creation date or refund ID, to ensure consistent
and predictable pagination output.
| QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | ||
| QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | ||
| QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | ||
| QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | ||
| QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make QueryDSL entity references static final fields.
QueryDSL entity references should be static final fields since they're constants and don't need to be instantiated per repository instance.
- QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity;
- QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity;
- QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity;
- QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity;
- QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity;
+ private static final QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity;
+ private static final QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity;
+ private static final QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity;
+ private static final QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity;
+ private static final QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | |
| QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | |
| QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | |
| QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | |
| QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; | |
| private static final QRefundJpaEntity refund = QRefundJpaEntity.refundJpaEntity; | |
| private static final QApplicationSchoolJpaEntity appSchool = QApplicationSchoolJpaEntity.applicationSchoolJpaEntity; | |
| private static final QApplicationJpaEntity application = QApplicationJpaEntity.applicationJpaEntity; | |
| private static final QProfileJpaEntity profile = QProfileJpaEntity.profileJpaEntity; | |
| private static final QPaymentJpaEntity payment = QPaymentJpaEntity.paymentJpaEntity; |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/admin/RefundQueryRepositoryImpl.java
around lines 23 to 27, the QueryDSL entity references are declared as instance
fields but should be static final fields because they are constants. Change the
declarations of refund, appSchool, application, profile, and payment to be
static final to avoid unnecessary instantiation for each repository instance.
polyglot-k
left a comment
polyglot-k
left a comment
There was a problem hiding this comment.
나머지는 우리 LLM 봇들이 너무 예쁘게 잘 달아줘서, LLM 리뷰 포함해서 이부분 보고 간단하게 고민 후 수정 부탁드려용 😎😎
| public List<RefundListResponse> getRefunds(int page, int size) { | ||
| Pageable pageable = PageRequest.of(page, size); | ||
| return refundQueryRepository.searchAllRefunds(pageable); | ||
| } |
There was a problem hiding this comment.
이거 Pagination 인거 같은데, 최대 몇 페이지인지, 다음 페이지 있는지 유무에 대한 메타데이터는 있어야할거 같아요
There was a problem hiding this comment.
음 다음 페이지에 대한 유무 부분에 대해서는 생각하지 못했네요! 감사합니당
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryDetailResponse.java (1)
70-71: Inconsistent date format in nested record example.The
InquiryAnswerDetailResponserecord'screatedAtexample uses timestamp format ("2025-07-10T10:00:00"), while the parent record uses date-only format ("2025-07-10"). This inconsistency might confuse API consumers.Consider updating the example to match the parent record format:
- @Schema(description = "작성일", example = "2025-07-10T10:00:00") + @Schema(description = "작성일", example = "2025-07-10")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/life/mosu/mosuserver/domain/admin/StudentQueryRepositoryImpl.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/inquiry/InquiryQueryRepositoryImpl.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/inquiry/InquiryStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentListResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryDetailResponse.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryResponse.java(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/life/mosu/mosuserver/domain/inquiry/InquiryStatus.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationExcelDto.java (1)
Schema(12-88)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentListResponse.java (1)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
Schema(8-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (10)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentListResponse.java (1)
21-21: LGTM: Consistent enum-to-string conversion for DTOsThe conversion from enum types (
Education,Grade) toStringtypes for theeducationLevelandgradefields aligns with the broader refactoring pattern seen across the codebase. This change improves API consistency and serialization handling while maintaining the semantic meaning through the updated repository mapping logic.Also applies to: 27-27
src/main/java/life/mosu/mosuserver/domain/admin/StudentQueryRepositoryImpl.java (3)
11-13: Good: Added necessary imports for enum handlingThe imports for
Education,Gender, andGradeenums are properly added to support the enhanced mapping logic in the repository methods.
124-127: Excellent: Null-safe enum-to-string conversion with custom gettersThe implementation properly extracts enum values into local variables and uses custom getter methods (
getGenderName(),getEducationName(),getGradeName()) with null safety checks. This approach provides meaningful string representations while maintaining type safety during the mapping process.Also applies to: 134-138
143-146: Consistent: Same pattern applied to Excel mappingThe Excel mapping method follows the same null-safe enum-to-string conversion pattern, ensuring consistency across different output formats.
Also applies to: 153-157
src/main/java/life/mosu/mosuserver/presentation/admin/dto/StudentExcelDto.java (1)
31-31: LGTM: Completing the consistent enum-to-string refactoringThe conversion of
educationLevelandgradefields from enum types toStringtypes completes the consistent refactoring pattern across all related DTOs. This change ensures uniform string representation in Excel exports while maintaining proper field annotations and documentation.Also applies to: 39-39
src/main/java/life/mosu/mosuserver/domain/inquiry/InquiryQueryRepositoryImpl.java (2)
3-3: Good improvement in code organization.Using static import for
formatDatecentralizes date formatting logic and improves code readability.
94-102: Excellent null handling and readability improvements.The changes enhance the code quality by:
- Extracting status to a local variable for better readability
- Adding proper null check with fallback to "N/A"
- Using centralized date formatting with
formatDate()- Consistent string representation for status using
getStatusName()These improvements make the code more robust and maintainable.
src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryResponse.java (2)
20-21: Good improvement in API usability.Changing the status field from enum to String with Korean labels improves the user experience and makes the API more frontend-friendly. The Swagger documentation is appropriately updated to reflect the new string values.
33-33: Correct implementation of status string conversion.Using
getStatusName()method properly converts the enum to its localized string representation, maintaining consistency with the field type change.src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryDetailResponse.java (1)
18-21: Good improvement in status representation and date format consistency.The changes to use String for status with Korean labels and the date-only format for createdAt example improve API consistency and user experience.
| inquiry.getStatus().getStatusName(), | ||
| inquiry.getCreatedAt(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential date formatting inconsistency.
The factory method uses inquiry.getCreatedAt() directly, while the repository implementation uses formatDate(). This could lead to inconsistent date formatting across the application.
Consider using the centralized date formatting:
- inquiry.getCreatedAt(),
+ formatDate(inquiry.getCreatedAt()),You'll need to add the static import:
+import static life.mosu.mosuserver.domain.base.BaseTimeEntity.formatDate;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| inquiry.getStatus().getStatusName(), | |
| inquiry.getCreatedAt(), | |
| // Add at the top of src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryDetailResponse.java | |
| import static life.mosu.mosuserver.domain.base.BaseTimeEntity.formatDate; | |
| // … inside your factory method … | |
| inquiry.getStatus().getStatusName(), | |
| formatDate(inquiry.getCreatedAt()), | |
| // … |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/inquiry/dto/InquiryDetailResponse.java
around lines 38 to 39, the code uses inquiry.getCreatedAt() directly, which may
cause inconsistent date formatting compared to other parts of the application
that use formatDate(). To fix this, replace inquiry.getCreatedAt() with a call
to the centralized formatDate() method to ensure consistent date formatting.
Also, add the necessary static import for formatDate() at the top of the file.
2 participants
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Chores